fix(plugin-auth): the auth catch-all yields paths better-auth does not own (#4088) - #4092
Merged
Merged
Conversation
…t own (#4088) `registerAuthRoutes` mounts `rawApp.all('${basePath}/*')` over the whole auth namespace (`/api/v1/auth` by default), and that handler was TERMINAL: it returned better-auth's response unconditionally, including the 404 better-auth produces for a path it does not implement. Any other plugin's route under that prefix was therefore reachable only if it happened to register FIRST — Hono runs handlers matching a path in registration order and the first to return a Response wins. That put a load-bearing surface at the mercy of `kernel.use()` order. `plugin-hono-server` mounts `/auth/me/permissions` and `/auth/me/localization` from its own `kernel:ready` hook (unconditionally since #4079); objectui's entire permission layer reads the former and `core/security/auth-gate.ts` allow-lists the latter as an endpoint a gated user MUST still reach to bootstrap the remediation UI. Register AuthPlugin before HonoServerPlugin and all of it silently 404s. Same class as #2567 and #4018: an invariant held by ordering luck rather than enforced. A 404 from better-auth now means "this path is not mine" and the catch-all yields, in either registration order. Deliberately narrow: only 404 falls through (401/403 are real answers, not disclaimers of ownership); precedence still favours the namespace owner, so better-auth wins every path it implements and only its leftovers are up for grabs; and when nothing downstream answers, better-auth's own 404 is returned verbatim so the unclaimed-path wire shape is unchanged. Two mechanics worth recording, both found the hard way: * `c.finalized` cannot discriminate "someone answered" from "nobody did" — reaching the end of the chain runs Hono's notFound handler, which sets a response and flips the flag. Status can: a non-404 downstream means someone answered. The trade is a downstream route's own 404 BODY (better-auth's is returned instead); status is identical and no route under this prefix answers 404 today. * The fall-back must ASSIGN `c.res`, not `return` the Response. The ADR-0069 D5 IP gate above registers `rawApp.use('${basePath}/*')`, so this chain contains middleware, and Hono's compose only assigns a handler's returned Response while `finalized` is false — a `return` here is silently dropped and the caller gets Hono's `404 Not Found` text instead of better-auth's JSON. Guard test drives a real Hono app with better-auth stubbed by a path table, registering the catch-all FIRST and the specific route SECOND — exactly the order that used to fail. Verified to bite: reverting the fix fails the two `/auth/me/*` cases. `hono` added as a plugin-auth devDependency so the test exercises the shipped handler rather than a stand-in. Checked for other instances of the shape: `runtime/dispatcher-plugin.ts` deliberately does NOT register an `/auth/*` wildcard, and the `/api/v1/*` catch-all in `adapters/hono` has no in-tree dependents. Cloud's `AuthProxyPlugin` may share the hazard but is out of this repo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VZLsKypjrGhiLpio2uWKu
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Contributor
Author
|
补上正文里说会贴的跨包结果 —— 本地全绿:
dogfood 那条是这次最实的验证:它驱动一遍真实的 OIDC 授权码流程,也就是真的走 better-auth 自己的端点、经过打了补丁的 catch-all。落穿只在 404 上发生,所以 owner 路径这条主干不受影响 —— 这条测试通过即为证。 Generated by Claude Code |
os-zhuang
marked this pull request as ready for review
July 30, 2026 09:40
This was referenced Jul 30, 2026
Closed
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…ire each to be classified (#4116) (#4122) A handler mounted on `<prefix>/*` claims an entire namespace, and Hono runs every handler matching a path in registration order with the first Response winning. So a TERMINAL wildcard makes every other route under that prefix reachable only when it happens to register first — and plugin registration order is not a contract anyone declares or tests. That shape has cost four fixes (#2567, #4018, #4088/#4092, cloud#923) and every one was found by hand, after the fact, by someone reading the code for an unrelated reason. The driven tests #4092 and cloud#923 shipped each pin ONE catch-all and cannot cover the next wildcard someone mounts. What never existed is the ENUMERATION. Adds it, following check-route-envelope.mjs (#3843) including the part that matters most: a site the scan finds but the ledger does not declare is an ERROR, not a default. Three states — `yields` (verified from the AST, so an entry cannot rot), `exempt` with a reason, `ratchet` naming the issue. AST rather than regex because "does this call next()?" is a question about parameter binding: a comment or string mentioning next(), or an inner closure's own next, all fool a textual match in the direction that PASSES while the defect ships. Found 13 namespace-claiming mounts where a manual grep had found 3, and with them two real, previously untracked instances of the #4088 defect in packages/adapters/hono — ratcheted under #4117. It also separates that file's deliberate `${prefix}/*` ownership (ADR-0076 OQ#9) from the two that merely forgot to yield; in source they look identical. Verified both directions by hand: reverting #4092's fix fails with the terminal diagnostic, and a new undeclared catch-all fails as NOT DECLARED.
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…oes not own, and unbreak the #4116 ledger (#4117) (#4133) Two things, and the second is urgent. ## The fix (#4117) `app.all(`${prefix}/auth/*`)` claimed a whole namespace and was TERMINAL: it returned the auth service's response unconditionally, including better-auth's 404 for a path it does not implement, and the legacy `handleAuth` bridge's own `handled: false` 404. That is #4088's shape. #4116's scan found it; the manual greps that preceded that scan had not. A 404 from better-auth, or `handled: false` from the dispatcher, now means "not this mount's path" and the handler yields. The predicate is the dispatcher's OWN `handled` flag wherever one exists — an explicit ownership answer beats inferring one from a status; only the better-auth hand-off lacks such a flag, and there the 404 is the signal, exactly as in #4092. What this buys here is narrower than #4092 and the code says so: it does NOT make a later-registered Hono route reachable, because the `${prefix}/*` dispatcher catch-all below is deliberately terminal (ADR-0076 OQ#9, #3576/#3608) and would swallow one either way. It means an unowned `/auth/*` path now reaches that gated `dispatch()` instead of dead-ending in a 404 built one mount earlier, so a domain handler registered for it becomes reachable — which is this adapter's actual extension mechanism. The first draft of the tests asserted the #4092 outcome and failed, which is how the distinction got established rather than assumed. ## Unbreaking main #4122 shipped the ledger declaring TWO ratcheted mounts here. Between its base (974c6d4) and its merge, #4087/#4112 landed and DELETED the `/storage/*` bridge — so the squash produced a main where the ledger declares a mount that no longer exists, and `check:wildcard-fallthrough` fails on main with "DECLARED but not found". That is my merge race, and this commit removes the entry. Worth recording why the guard behaved well here: the stale-entry arm is what caught it, immediately, on the first run against the new main. And #4112 reached the same verdict about that wildcard independently, from the other direction — "the wildcard was wider than the two routes it served". Two independent reads landing on one defect is the argument for enumerating the shape rather than finding it by eye each time. Also extends `callsContinuation`: handing the continuation to a helper (`yieldUnowned(c, next, …)`) now counts as yielding. Without that the checker called this fix terminal — a false negative that would have pushed the compose subtlety toward being duplicated at each call site instead of written once. Guard: 7 yielding / 0 ratcheted / 5 exempt over 12 mounts. Self-test 17 cases. adapters/hono 73 passed. Verified both defences bite: reverting the fix fails the guard with the TERMINAL diagnostic and fails 2 of the 6 new tests. The 3 `tsc` errors and 2 `eslint` rule-definition errors in this package are pre-existing, measured identical on a clean tree. Claude-Session: https://claude.ai/code/session_013VZLsKypjrGhiLpio2uWKu Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4088。#4073 / #4079 实施中发现的加载顺序隐患。
问题
plugin-auth 在自己的
kernel:readyhook 里挂一个覆盖整个 auth 命名空间的 catch-all(/api/v1/auth/*),而它是终结式的 —— 无条件返回 better-auth 的响应,包括 better-auth 不认识该路径时的 404。于是别的插件挂在这个前缀下的路由,只有恰好注册得更早才能被访问到(Hono 对同一路径按注册顺序执行 handler,先返回 Response 的赢)。这让一个承重面完全取决于
kernel.use()顺序:/auth/me/permissionsos serve)而 objectui console 的整个权限层读
/auth/me/permissions,core/security/auth-gate.ts把/me/localization列为"被门禁拦住的用户必须仍能访问"。这与 #2567、#4018 是同一类:不变量靠加载顺序侥幸成立而非被强制。更一般地,任何插件想在/api/v1/auth/*下挂路由今天都会被吞掉。修法
better-auth 返回 404 即视为"这条路径不是我的",落穿给其他匹配者 —— 与注册顺序无关。刻意收窄:
404 Not Found。两处机制细节值得记下,都是踩出来的(已订正到 issue,那里原本的建议补丁两条都写错了):
c.finalized不能用作判据。 链末尾无人匹配时 Hono 会运行 notFound handler,它设置响应并把该标志翻真,因此分不清"有人接手"与"没人接手"。改用状态码:下游非 404 才算有人接手。代价是下游自己那份 404 body 会被 better-auth 的取代(status 相同),而这个前缀下今天没有任何路由回 404。c.res = response,不能return response。 ADR-0069 D5 的 IP 门禁在上方注册了rawApp.use('${basePath}/*'),链里因此有中间件;Hono 的 compose 只在finalized为 false 时才把 handler 的返回值赋给c.res,而 notFound 已把它翻真 ——return会被静默丢弃,调用方拿到 Hono 的404 Not Found文本。验证
新增
auth-catchall-fallthrough.test.ts(6 例):打真实 Hono app、better-auth 用一张 path → Response 表替身(这样"哪些路径属于 owner"完全可控),并且 catch-all 先注册、具体路由后注册 —— 正是过去会失败的顺序。已验证会咬:
git stash撤掉修正后重跑,/auth/me/permissions与/auth/me/localization两条立刻失败(拿到 better-auth 的 404);其余 4 条是不变量保护(owner 路径不被劫持、401 不落穿、无人认领时 404 逐字保留、每请求只转发一次),两种情况下都应通过。为此给 plugin-auth 加了
honodevDependency —— 这个测试的价值就在于打真实 Hono 链,换成手写替身等于测假货。plugin-auth全量tsc --noEmit+eslintclient/plugin-hono-server/http-conformance/rest-auth-gate/ dogfood OIDC 授权码流程本地仍在跑(spec的 DTS 构建是瓶颈),结果我会补在评论里;CI 也覆盖这些。顺带核实:这个形状没有第二处落在
os serve路径上runtime/src/dispatcher-plugin.ts有明确注释说它故意不注册/auth/*通配(better-auth 的原生Response无法经IHttpServer.send干净表达);adapters/hono/src/index.ts:382的app.all('${prefix}/*')确实是覆盖/api/v1/*的终结式 catch-all,但全仓无任何包依赖@objectstack/adapter-hono,其形状另有 ADR-0076 OQ#9 / docs(adr): ADR-0076 OQ#9 裁决 — catch-all 洗白、步骤①吸收完成 (#2462) #3576 / runtime: adapter 枚举 DomainHandlerRegistry 挂显式路由(裁决:有意不排期) #3608 的既定结论;AuthProxyPlugin同样直接在 raw Hono app 上挂/auth/*通配。若它也是终结式,cloud 部署有同一隐患 ——cloud不在本会话仓库范围内,已记为独立跟进。Generated by Claude Code